Export: trimming enhancements#3530
Conversation
setting subincludes at package level instead of at target level
- register subinclude statements in the package metadata - filter subincludes label - export all non build_target related statements
…dary build def with sources the updated test uses non-standard child naming to validate new trimming logic
…s. Add fatal to no-op implementation
- export and trim for stmts: implementation and tests - remove unused indent level - trim if-else stmt by keeping clauses but writing a pass those not required - rework export to trim per stmt and include extra parsing info for if-else stmt
reworked activeSubincludes compute by relying on new logic
…ng export added a test case
successful export of go_binary in complex repo
| return func() *core.BuildStatement { | ||
| stmtScope := s | ||
| for curr := s; curr != nil; curr = curr.callerScope { | ||
| if curr.pkg != nil && curr.filename == s.pkg.Filename { |
There was a problem hiding this comment.
I think your test for CurrentBuildStatement is fine, as it's relatively understandable what the test data represents. I'm less sure about the ActiveSubincludes test, which has a lot more logic and it's less clear what's going on.
In general, I'm more in favour of setting up a test repo with BUILD files/defs which actually get parsed, as that's much closer to the public interface to the interpreter/parser. Injecting some custom native code for testing actually seems like quite an elegant solution to me - it would effectively be some sort of assertion function?
| standaloneScope := &scope{metadata: newScopeMetadata()} | ||
| standaloneScope.metadata.SetCursor(rootStmt) |
There was a problem hiding this comment.
For consistency, let's move this out of the subtest
| if state.Cache != nil { | ||
| state.Cache.Shutdown() | ||
| } | ||
| if state.RemoteClient != nil { |
There was a problem hiding this comment.
out of curiosity, why has this moved?
| } | ||
|
|
||
| // CleanUp cleans up and shuts down the build state. | ||
| func CleanUp(state *core.BuildState) { |
There was a problem hiding this comment.
nit: CleanUp -> Cleanup
| Test bool | ||
| IsQuery bool | ||
| ParseMetadata bool | ||
| // Keep the workers running in the background for inline parsing during specific ops (e.g. export). |
There was a problem hiding this comment.
nit: ops -> operations
Similarly op below. No need to abbreviate comments
Enhancements to plz export, moving from a basic target-level trimming (using gc.RewriteFile) to build statement-level trimming, including only the required build rules and subincludes.
For consistency, we format all the exported BUILD files.
Changelog:
src/export/export.goto enforce better separation of the DefaultExporter (for trimming) and NoTrimExporter.